Make errors support verbose formatting to print wrapped errors #1396
Conversation
This makes it possible to print the entire hierarchy when using `%+v` - in particular, when running tests this will show causes and stack traces, not just the error message, which can help debugging tests.
| { | ||
| name: "WorkflowExecutionError", | ||
| err: NewWorkflowExecutionError("wid", "rid", "workflowType", newWorkflowPanicError("test message", "stack trace")), | ||
| expected: "stack trace\ntest message\nworkflow execution error (type: workflowType, workflowID: wid, runID: rid): test message", |
There was a problem hiding this comment.
It seems odd to show inner error information before outer error information
There was a problem hiding this comment.
I basically copied the behavior from github.com/pkg/errors - I did also think the same but for consistency it seemed like the best approach
| @@ -820,6 +845,24 @@ func (e *WorkflowExecutionError) Unwrap() error { | |||
| return e.cause | |||
| } | |||
|
|
|||
| func formatError(e error, s fmt.State, verb rune, verboseValue any) { | |||
There was a problem hiding this comment.
I fear the future proof-ness of this. If Go adds new print flags that'd work for errors, we'd silently not support them.
I wonder if it'd be better to change where you're formatting these errors to handle more advanced types instead of adding a formatting feature to a few of the errors here. I think that gives you the most flexibility instead of assuming exactly what people want when %+v is put (you want stack trace, but another might want struct fields). We tend to put accessors on the errors for users to extract and display the way they prefer.
(having said that, I'm not completely against the change, I think it just makes too many assumptions of what users may want to see instead of encouraging them to extract the information themselves when they're displaying)
There was a problem hiding this comment.
You're right it might be different what people want - I just copied the behavior from pkg/errors since that seems to be what a lot of people (including myself) are used to. I'd personally say that %+v means max verbose, so dump anything that can be dumped, but again that might be a personal preference...
This makes it possible to print the entire hierarchy when using
%+v- in particular, when running tests this will show causes and stack traces, not just the error message, which can help debugging tests.Closes #1395